Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Camera Index Assignment. #1031

Merged
merged 9 commits into from
Dec 1, 2023
Merged

Fix Camera Index Assignment. #1031

merged 9 commits into from
Dec 1, 2023

Conversation

BytingBulldogs3539
Copy link
Contributor

Fixes a bug where multiple cameras would receive identical stream indexes and would cause at least one camera to not function at all and the other to not display a stream.

appears that the remove method from arraylist was not functioning properly.
@BytingBulldogs3539 BytingBulldogs3539 requested a review from a team as a code owner November 29, 2023 00:17
@mcm001
Copy link
Contributor

mcm001 commented Nov 29, 2023

Can we add a unit test that fails with the old code and passes with the new code?

Cleaner fix to the bug of multiple cameras of the same type aka dual arducam ov9281 uc844 not working. The past equals would result in false positives.
@BytingBulldogs3539
Copy link
Contributor Author

Can we add a unit test that fails with the old code and passes with the new code?

Reverted code back to original as the issue was truly with the equals case of the usb camera source class giving false positives. There is no easy/good way to update the unit test to mock a usb camera that much.

@viggy96
Copy link

viggy96 commented Nov 29, 2023

Tested this build on Linux x64, works well with 3 identical Arducam cameras.

@srimanachanta
Copy link
Member

Can we add a unit test that fails with the old code and passes with the new code?

Reverted code back to original as the issue was truly with the equals case of the usb camera source class giving false positives. There is no easy/good way to update the unit test to mock a usb camera that much.

Right, make two USBCameraSource with the same characteristics of the Arducams that are currently causing the issue and test the equality method. If it needs to be updated, it previously didn't work, a unit test should show that.

One of the reasons the that usbcamerasource equals method used to incorrectly determine equality was because the quirkycamera object didn't have a basename. When a camera that had a quirk was found it would set equal to a predefined quirkycamera without changing the name first. Add unit test that will better test the equality of two usbcamerasources. This required a few changes to allow creating a fake usbcamerasource.
@mcm001 mcm001 merged commit 469bc0e into PhotonVision:master Dec 1, 2023
22 checks passed
@mdurrani808
Copy link
Contributor

@viggy96 Just to confirm, with this PR you were able to use 3 identical cameras without renaming? Or you still needed to rename?

@viggy96
Copy link

viggy96 commented Dec 1, 2023

I renamed each camera using the Arducam utility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants